From be8dc153762406dc90d297b4ad2c7427ab000d0d Mon Sep 17 00:00:00 2001 From: "kaf24@firebug.cl.cam.ac.uk" Date: Thu, 24 Nov 2005 15:38:33 +0100 Subject: [PATCH] Clean up mark_dirty() implementation to check for log-dirty status internally, with the shadow_lock held for consistency. Callers no need to check log-dirty status themselves. Also, add dirty logging to alloc_page_type/free_page_type, so that we see page tables coming and going during live relocation. Signed-off-by: Keir Fraser --- xen/arch/x86/mm.c | 31 +++++------ xen/arch/x86/shadow.c | 20 ++----- xen/arch/x86/shadow32.c | 19 +------ xen/arch/x86/shadow_public.c | 9 ---- xen/include/asm-x86/domain.h | 2 - xen/include/asm-x86/grant_table.h | 6 +-- xen/include/asm-x86/shadow.h | 89 +++++++------------------------ 7 files changed, 41 insertions(+), 135 deletions(-) diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 82d8093bd5..31ec92fe42 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -1289,6 +1289,11 @@ static int mod_l4_entry(l4_pgentry_t *pl4e, int alloc_page_type(struct pfn_info *page, unsigned long type) { + struct domain *owner = page_get_owner(page); + + if ( owner != NULL ) + mark_dirty(owner, page_to_pfn(page)); + switch ( type & PGT_type_mask ) { case PGT_l1_page_table: @@ -1318,16 +1323,14 @@ void free_page_type(struct pfn_info *page, unsigned long type) struct domain *owner = page_get_owner(page); unsigned long gpfn; - if ( owner != NULL ) + if ( unlikely((owner != NULL) && shadow_mode_enabled(owner)) ) { + mark_dirty(owner, page_to_pfn(page)); if ( unlikely(shadow_mode_refcounts(owner)) ) return; - if ( unlikely(shadow_mode_enabled(owner)) ) - { - gpfn = __mfn_to_gpfn(owner, page_to_pfn(page)); - ASSERT(VALID_M2P(gpfn)); - remove_shadow(owner, gpfn, type & PGT_type_mask); - } + gpfn = __mfn_to_gpfn(owner, page_to_pfn(page)); + ASSERT(VALID_M2P(gpfn)); + remove_shadow(owner, gpfn, type & PGT_type_mask); } switch ( type & PGT_type_mask ) @@ -2203,8 +2206,7 @@ int do_mmu_update( { shadow_lock(d); - if ( shadow_mode_log_dirty(d) ) - __mark_dirty(d, mfn); + __mark_dirty(d, mfn); if ( page_is_page_table(page) && !page_out_of_sync(page) ) @@ -2263,13 +2265,7 @@ int do_mmu_update( set_pfn_from_mfn(mfn, gpfn); okay = 1; - /* - * If in log-dirty mode, mark the corresponding - * page as dirty. - */ - if ( unlikely(shadow_mode_log_dirty(FOREIGNDOM)) && - mark_dirty(FOREIGNDOM, mfn) ) - FOREIGNDOM->arch.shadow_dirty_block_count++; + mark_dirty(FOREIGNDOM, mfn); put_page(&frame_table[mfn]); break; @@ -2841,8 +2837,7 @@ long do_update_descriptor(u64 pa, u64 desc) { shadow_lock(dom); - if ( shadow_mode_log_dirty(dom) ) - __mark_dirty(dom, mfn); + __mark_dirty(dom, mfn); if ( page_is_page_table(page) && !page_out_of_sync(page) ) shadow_mark_mfn_out_of_sync(current, gpfn, mfn); diff --git a/xen/arch/x86/shadow.c b/xen/arch/x86/shadow.c index daad0b68d9..c8ec769596 100644 --- a/xen/arch/x86/shadow.c +++ b/xen/arch/x86/shadow.c @@ -1858,8 +1858,7 @@ static inline int l1pte_write_fault( SH_VVLOG("l1pte_write_fault: updating spte=0x%" PRIpte " gpte=0x%" PRIpte, l1e_get_intpte(spte), l1e_get_intpte(gpte)); - if ( shadow_mode_log_dirty(d) ) - __mark_dirty(d, gmfn); + __mark_dirty(d, gmfn); if ( mfn_is_page_table(gmfn) ) shadow_mark_va_out_of_sync(v, gpfn, gmfn, va); @@ -2021,9 +2020,7 @@ static int shadow_fault_32(unsigned long va, struct cpu_user_regs *regs) domain_crash_synchronous(); } - // if necessary, record the page table page as dirty - if ( unlikely(shadow_mode_log_dirty(d)) ) - __mark_dirty(d, __gpfn_to_mfn(d, l2e_get_pfn(gpde))); + __mark_dirty(d, __gpfn_to_mfn(d, l2e_get_pfn(gpde))); } shadow_set_l1e(va, spte, 1); @@ -2082,8 +2079,7 @@ static int do_update_va_mapping(unsigned long va, * the PTE in the PT-holding page. We need the machine frame number * for this. */ - if ( shadow_mode_log_dirty(d) ) - __mark_dirty(d, va_to_l1mfn(v, va)); + __mark_dirty(d, va_to_l1mfn(v, va)); shadow_unlock(d); @@ -3189,11 +3185,7 @@ static inline int l2e_rw_fault( l1e_remove_flags(sl1e, _PAGE_RW); } } else { - /* log dirty*/ - /* - if ( shadow_mode_log_dirty(d) ) - __mark_dirty(d, gmfn); - */ + /* __mark_dirty(d, gmfn); */ } // printk("<%s> gpfn: %lx, mfn: %lx, sl1e: %lx\n", __func__, gpfn, mfn, l1e_get_intpte(sl1e)); /* The shadow entrys need setup before shadow_mark_va_out_of_sync()*/ @@ -3476,9 +3468,7 @@ check_writeable: if (unlikely(!__guest_set_l1e(v, va, &gl1e))) domain_crash_synchronous(); - // if necessary, record the page table page as dirty - if ( unlikely(shadow_mode_log_dirty(d)) ) - __mark_dirty(d, __gpfn_to_mfn(d, l2e_get_pfn(gl2e))); + __mark_dirty(d, __gpfn_to_mfn(d, l2e_get_pfn(gl2e))); } shadow_set_l1e_64(va, (pgentry_64_t *)&sl1e, 1); diff --git a/xen/arch/x86/shadow32.c b/xen/arch/x86/shadow32.c index bcbbdd5cc2..5636840f70 100644 --- a/xen/arch/x86/shadow32.c +++ b/xen/arch/x86/shadow32.c @@ -1274,8 +1274,6 @@ static int shadow_mode_table_op( d->arch.shadow_fault_count = 0; d->arch.shadow_dirty_count = 0; - d->arch.shadow_dirty_net_count = 0; - d->arch.shadow_dirty_block_count = 0; break; @@ -1284,13 +1282,9 @@ static int shadow_mode_table_op( sc->stats.fault_count = d->arch.shadow_fault_count; sc->stats.dirty_count = d->arch.shadow_dirty_count; - sc->stats.dirty_net_count = d->arch.shadow_dirty_net_count; - sc->stats.dirty_block_count = d->arch.shadow_dirty_block_count; d->arch.shadow_fault_count = 0; d->arch.shadow_dirty_count = 0; - d->arch.shadow_dirty_net_count = 0; - d->arch.shadow_dirty_block_count = 0; if ( (sc->dirty_bitmap == NULL) || (d->arch.shadow_dirty_bitmap == NULL) ) @@ -1327,9 +1321,6 @@ static int shadow_mode_table_op( case DOM0_SHADOW_CONTROL_OP_PEEK: sc->stats.fault_count = d->arch.shadow_fault_count; sc->stats.dirty_count = d->arch.shadow_dirty_count; - sc->stats.dirty_net_count = d->arch.shadow_dirty_net_count; - sc->stats.dirty_block_count = d->arch.shadow_dirty_block_count; - if ( (sc->dirty_bitmap == NULL) || (d->arch.shadow_dirty_bitmap == NULL) ) @@ -2738,9 +2729,7 @@ int shadow_fault(unsigned long va, struct cpu_user_regs *regs) domain_crash_synchronous(); } - // if necessary, record the page table page as dirty - if ( unlikely(shadow_mode_log_dirty(d)) ) - __mark_dirty(d, __gpfn_to_mfn(d, l2e_get_pfn(gpde))); + __mark_dirty(d, __gpfn_to_mfn(d, l2e_get_pfn(gpde))); } shadow_set_l1e(va, spte, 1); @@ -2837,8 +2826,6 @@ int shadow_do_update_va_mapping(unsigned long va, shadow_lock(d); - //printk("%s(va=%p, val=%p)\n", __func__, (void *)va, (void *)l1e_get_intpte(val)); - // This is actually overkill - we don't need to sync the L1 itself, // just everything involved in getting to this L1 (i.e. we need // linear_pg_table[l1_linear_offset(va)] to be in sync)... @@ -2853,10 +2840,8 @@ int shadow_do_update_va_mapping(unsigned long va, * the PTE in the PT-holding page. We need the machine frame number * for this. */ - if ( shadow_mode_log_dirty(d) ) - __mark_dirty(d, va_to_l1mfn(v, va)); + __mark_dirty(d, va_to_l1mfn(v, va)); -// out: shadow_unlock(d); return rc; diff --git a/xen/arch/x86/shadow_public.c b/xen/arch/x86/shadow_public.c index 36454f9e51..cc91811e34 100644 --- a/xen/arch/x86/shadow_public.c +++ b/xen/arch/x86/shadow_public.c @@ -1183,8 +1183,6 @@ static int shadow_mode_table_op( d->arch.shadow_fault_count = 0; d->arch.shadow_dirty_count = 0; - d->arch.shadow_dirty_net_count = 0; - d->arch.shadow_dirty_block_count = 0; break; @@ -1193,15 +1191,10 @@ static int shadow_mode_table_op( sc->stats.fault_count = d->arch.shadow_fault_count; sc->stats.dirty_count = d->arch.shadow_dirty_count; - sc->stats.dirty_net_count = d->arch.shadow_dirty_net_count; - sc->stats.dirty_block_count = d->arch.shadow_dirty_block_count; d->arch.shadow_fault_count = 0; d->arch.shadow_dirty_count = 0; - d->arch.shadow_dirty_net_count = 0; - d->arch.shadow_dirty_block_count = 0; - if ( (sc->dirty_bitmap == NULL) || (d->arch.shadow_dirty_bitmap == NULL) ) { @@ -1236,8 +1229,6 @@ static int shadow_mode_table_op( case DOM0_SHADOW_CONTROL_OP_PEEK: sc->stats.fault_count = d->arch.shadow_fault_count; sc->stats.dirty_count = d->arch.shadow_dirty_count; - sc->stats.dirty_net_count = d->arch.shadow_dirty_net_count; - sc->stats.dirty_block_count = d->arch.shadow_dirty_block_count; if ( (sc->dirty_bitmap == NULL) || (d->arch.shadow_dirty_bitmap == NULL) ) diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h index ed05d32c81..94111fe1d1 100644 --- a/xen/include/asm-x86/domain.h +++ b/xen/include/asm-x86/domain.h @@ -49,8 +49,6 @@ struct arch_domain unsigned int shadow_fault_count; unsigned int shadow_dirty_count; - unsigned int shadow_dirty_net_count; - unsigned int shadow_dirty_block_count; /* full shadow mode */ struct out_of_sync_entry *out_of_sync; /* list of out-of-sync pages */ diff --git a/xen/include/asm-x86/grant_table.h b/xen/include/asm-x86/grant_table.h index bd6eb09fc3..38f46b69c9 100644 --- a/xen/include/asm-x86/grant_table.h +++ b/xen/include/asm-x86/grant_table.h @@ -33,10 +33,6 @@ int steal_page_for_grant_transfer( #define gnttab_shared_mfn(d, t, i) \ ((virt_to_phys((t)->shared) >> PAGE_SHIFT) + (i)) -#define gnttab_log_dirty(d, f) \ - do { \ - if ( unlikely(shadow_mode_log_dirty((d))) ) \ - mark_dirty((d), (f)); \ - } while ( 0 ) +#define gnttab_log_dirty(d, f) mark_dirty((d), (f)) #endif /* __ASM_GRANT_TABLE_H__ */ diff --git a/xen/include/asm-x86/shadow.h b/xen/include/asm-x86/shadow.h index 5d82713cae..d413035468 100644 --- a/xen/include/asm-x86/shadow.h +++ b/xen/include/asm-x86/shadow.h @@ -468,21 +468,18 @@ static inline void shadow_put_page(struct domain *d, /************************************************************************/ -static inline int __mark_dirty(struct domain *d, unsigned long mfn) +static inline void __mark_dirty(struct domain *d, unsigned long mfn) { unsigned long pfn; - int rc = 0; ASSERT(shadow_lock_is_acquired(d)); - ASSERT(d->arch.shadow_dirty_bitmap != NULL); - if ( !VALID_MFN(mfn) ) - return rc; + if ( likely(!shadow_mode_log_dirty(d)) || !VALID_MFN(mfn) ) + return; - // N.B. This doesn't use __mfn_to_gpfn(). - // This wants the nice compact set of PFNs from 0..domain's max, - // which __mfn_to_gpfn() only returns for translated domains. - // + ASSERT(d->arch.shadow_dirty_bitmap != NULL); + + /* We /really/ mean PFN here, even for non-translated guests. */ pfn = get_pfn_from_mfn(mfn); /* @@ -491,16 +488,13 @@ static inline int __mark_dirty(struct domain *d, unsigned long mfn) * Nothing to do here... */ if ( unlikely(IS_INVALID_M2P_ENTRY(pfn)) ) - return rc; + return; - if ( likely(pfn < d->arch.shadow_dirty_bitmap_size) ) + /* N.B. Can use non-atomic TAS because protected by shadow_lock. */ + if ( likely(pfn < d->arch.shadow_dirty_bitmap_size) && + !__test_and_set_bit(pfn, d->arch.shadow_dirty_bitmap) ) { - /* N.B. Can use non-atomic TAS because protected by shadow_lock. */ - if ( !__test_and_set_bit(pfn, d->arch.shadow_dirty_bitmap) ) - { - d->arch.shadow_dirty_count++; - rc = 1; - } + d->arch.shadow_dirty_count++; } #ifndef NDEBUG else if ( mfn < max_page ) @@ -513,18 +507,17 @@ static inline int __mark_dirty(struct domain *d, unsigned long mfn) frame_table[mfn].u.inuse.type_info ); } #endif - - return rc; } -static inline int mark_dirty(struct domain *d, unsigned int mfn) +static inline void mark_dirty(struct domain *d, unsigned int mfn) { - int rc; - shadow_lock(d); - rc = __mark_dirty(d, mfn); - shadow_unlock(d); - return rc; + if ( unlikely(shadow_mode_log_dirty(d)) ) + { + shadow_lock(d); + __mark_dirty(d, mfn); + shadow_unlock(d); + } } @@ -566,8 +559,7 @@ __guest_set_l2e( if ( unlikely(shadow_mode_translate(d)) ) update_hl2e(v, va); - if ( unlikely(shadow_mode_log_dirty(d)) ) - __mark_dirty(d, pagetable_get_pfn(v->arch.guest_table)); + __mark_dirty(d, pagetable_get_pfn(v->arch.guest_table)); } static inline void @@ -787,8 +779,7 @@ static inline int l1pte_write_fault( SH_VVLOG("l1pte_write_fault: updating spte=0x%" PRIpte " gpte=0x%" PRIpte, l1e_get_intpte(spte), l1e_get_intpte(gpte)); - if ( shadow_mode_log_dirty(d) ) - __mark_dirty(d, gmfn); + __mark_dirty(d, gmfn); if ( mfn_is_page_table(gmfn) ) shadow_mark_va_out_of_sync(v, gpfn, gmfn, va); @@ -1325,46 +1316,6 @@ shadow_max_pgtable_type(struct domain *d, unsigned long gpfn, return pttype; } -/* - * N.B. We can make this locking more fine grained (e.g., per shadow page) if - * it ever becomes a problem, but since we need a spin lock on the hash table - * anyway it's probably not worth being too clever. - */ -static inline unsigned long get_shadow_status( - struct domain *d, unsigned long gpfn, unsigned long stype) -{ - unsigned long res; - - ASSERT(shadow_mode_enabled(d)); - - /* - * If we get here we know that some sort of update has happened to the - * underlying page table page: either a PTE has been updated, or the page - * has changed type. If we're in log dirty mode, we should set the - * appropriate bit in the dirty bitmap. - * N.B. The VA update path doesn't use this and is handled independently. - * - * XXX need to think this through for vmx guests, but probably OK - */ - - shadow_lock(d); - - if ( shadow_mode_log_dirty(d) ) - __mark_dirty(d, __gpfn_to_mfn(d, gpfn)); - - if ( !(res = __shadow_status(d, gpfn, stype)) ) - shadow_unlock(d); - - return res; -} - - -static inline void put_shadow_status(struct domain *d) -{ - shadow_unlock(d); -} - - static inline void delete_shadow_status( struct domain *d, unsigned long gpfn, unsigned long gmfn, unsigned int stype) { -- 2.30.2